Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make it easier to add a hint or an error message to a fieldset #1281

Closed
wants to merge 9 commits into from
Closed

Make it easier to add a hint or an error message to a fieldset #1281

wants to merge 9 commits into from

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Apr 23, 2019 •
edited
Loading

This feature adds a new macro govukFormGroup() to resolve #1166.

Because govukFieldset() is a wrapper only, it's up to services to manage the hint and error message markup themselves when making more complex multi-field pages.

Duplicated inside almost all form control macros is Nunjucks code to generate a govukLabel() label, a govukHint() hint an a govukErrorMessage() error message:

  {{ govukLabel({
    html: params.label.html,
    text: params.label.text,
    classes: params.label.classes,
    isPageHeading: params.label.isPageHeading,
    attributes: params.label.attributes,
    for: params.id
  }) | indent(2) | trim }}
{% if params.hint %}
  {% set hintId = params.id + '-hint' %}
  {% set describedBy = describedBy + ' ' + hintId if describedBy else hintId %}
  {{ govukHint({
    id: hintId,
    classes: params.hint.classes,
    attributes: params.hint.attributes,
    html: params.hint.html,
    text: params.hint.text
  }) | indent(2) | trim }}
{% endif %}
{% if params.errorMessage %}
  {% set errorId = params.id + '-error' %}
  {% set describedBy = describedBy + ' ' + errorId if describedBy else errorId %}
  {{ govukErrorMessage({
    id: errorId,
    classes: params.errorMessage.classes,
    attributes: params.errorMessage.attributes,
    html: params.errorMessage.html,
    text: params.errorMessage.text,
    visuallyHiddenText: params.errorMessage.visuallyHiddenText
  }) | indent(2) | trim }}
{% endif %}

It's used by textarea, select, radios, checkboxes, text input, file upload and date input.

This allows us to port existing services with more complex multi-field pages (often with some fields nested inside other fieldsets) like this:

Example

With little markup like:

{% call govukFormGroup({
  fieldset: {
    legend: {
      text: "Pick one of the following options:",
      classes: "govuk-fieldset__legend--m"
    }
  }
}) %}

{{ govukRadios({
  "fieldset": {
    "legend": {
      text: "Group 1",
      classes: "govuk-fieldset__legend--s"
    }
  },
  "idPrefix": "radio-stacked-sub-headings-group-1",
  "name": "radio-stacked-sub-headings",
  "items": [
    {
      "value": "a",
      "text": "Option A"
    },
    {
      "value": "b",
      "text": "Option B"
    },
    {
      "value": "c",
      "text": "Option C"
    }
  ]
}) }}

{{ govukRadios({
  "fieldset": {
    "legend": {
      text: "Group 2",
      classes: "govuk-fieldset__legend--s"
    }
  },
  "idPrefix": "radio-stacked-sub-headings-group-2",
  "name": "radio-stacked-sub-headings",
  "items": [
    {
      "value": "a",
      "text": "Option A"
    },
    {
      "value": "b",
      "text": "Option B"
    },
    {
      "value": "c",
      "text": "Option C"
    }
  ]
}) }}

{{ govukRadios({
  "fieldset": {
    "legend": {
      text: "Group 3",
      classes: "govuk-fieldset__legend--s"
    }
  },
  "idPrefix": "radio-stacked-sub-headings-group-3",
  "name": "radio-stacked-sub-headings",
  "items": [
    {
      "value": "a",
      "text": "Option A"
    },
    {
      "value": "b",
      "text": "Option B"
    },
    {
      "value": "c",
      "text": "Option C"
    }
  ]
}) }}

{% endcall %}

This merge request demonstrates how we might remove the management of hints and error messages making new Nunjucks macros and form layouts a little easier to write in future.

@colinrotherham colinrotherham mentioned this pull request Apr 23, 2019
Closed
@colinrotherham colinrotherham force-pushed the form-group-wrapper branch 2 times, most recently from a9367dc to 839189a Compare April 23, 2019 13:29
@colinrotherham
Copy link
Contributor Author

This change moves:

govuk/objects/form-group

Into a macro:

govuk/component/form-group

So will need documenting here (to be created):
https://design-system.service.gov.uk/components/form-group

@NickColley
Copy link
Contributor

NickColley commented Apr 30, 2019 •
edited
Loading

Hey Colin,

We are currently prioritising work to make GOV.UK Frontend pass WCAG 2.1 requirements and we have an external audit soon.

This work looks really well done, we want to discuss it as a team and make sure it's the right approach, and then give some proper feedback.

We're not going to be able to follow up on this until we've done the audit so thanks a lot for your patience. :)

Nick

@NickColley NickColley added the awaiting triage Needs triaging by team label Apr 30, 2019
@colinrotherham colinrotherham force-pushed the form-group-wrapper branch 3 times, most recently from 0632f0f to 11bd792 Compare May 15, 2019 11:24
@colinrotherham
Copy link
Contributor Author

Moved the aria-describedby param changes separately into #1347

@kellylee-gds kellylee-gds removed the awaiting triage Needs triaging by team label May 15, 2019
@colinrotherham colinrotherham force-pushed the form-group-wrapper branch 3 times, most recently from db3c4a7 to 5746c6b Compare May 28, 2019 10:42
@NickColley NickColley added the awaiting triage Needs triaging by team label Jun 13, 2019
@kellylee-gds kellylee-gds removed the awaiting triage Needs triaging by team label Jun 19, 2019
@colinrotherham colinrotherham changed the title Remove duplicate form field markup from each macro Make it easier to add a hint or an error message to a fieldset Jun 21, 2019
@36degrees
Copy link
Contributor

Hey @colinrotherham,

Thanks again for your work on this.

This change makes a lot of sense, but because of the limitations of Nunjucks it ends up breaking a lot of the conventions we’ve built for how components should work. For that reason, we're going to close it.

We have got an epic next quarter to look at improving the way the macros work, and we’ll definitely be looking to do something similar if we can work out how to make it fit (for example, by finding a way to do merge objects)

Thanks for your patience - we’ve been somewhat preoccupied with the work on 3.0, but we need to work out how to better handle contributions like this so we don’t leave people hanging.

Ollie

@36degrees 36degrees closed this Jun 21, 2019
@NickColley NickColley mentioned this pull request Dec 3, 2019
Closed
@colinrotherham colinrotherham mentioned this pull request Aug 8, 2023
Merged
@colinrotherham colinrotherham mentioned this pull request Jan 30, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make it easier to add a hint or an error message to a fieldset
4 participants
@colinrotherham @NickColley @36degrees @kellylee-gds